[DON'T MERGE] LITE-29320 LITE-29322 Draft for XVS extension enhanced filtering and … #138
Conversation
|
Ivan Grebenshchikov seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| listings = get_all_listing_info(client) | ||
| vendors = [li['vendor'] for li in listings] | ||
| hubs = [hub['hub'] for li in listings for hub in li['contract']['marketplace']['hubs']] | ||
|
|
There was a problem hiding this comment.
There is a cost of custom cases
|
I agree with you that the 1st one seems to be the best one for now. |
There was a problem hiding this comment.
Yes, agree with you both. Option 1 seems to be the most suitable one taking into account what we have now, without doing extra efforts and knowing it limitations. The idea to have a mixin or to reuse code in some way that we can have it inside our future "extension utilities" library for me sound really good. If we document this in a way that the user can understand the configuration and the corner cases of the filters based on this library will work. In the meanwhile we can investigate how to solve the documented limitations and/or go with opt 3.
Option 3:
Put that way it sounds the most convenient opt, but the question is how we can do that?, what are the options that we have if we remove fastapi-filter lib without writing all from scratch. For example, sqlalchemy-mixins offer an utility to use "django-like" filters that is called smartquery
schema = {
Comment.post: {
Post.user: JOINED
}
}
query = db.query(Comment) # could be any query you want
res = smart_query(query,
filters={
'post___public': True,
'user__isnull': False
},
sort_attrs=['user___name', '-created_at'],
schema=schema).all()
Maybe we are not that far of combining this two, without removing the fastapi-filter library, add support to specify related fields, and let smartquery do its magic. But keep in mind that this is case of study and just an idea.
…sort
Just an example of implementation filtering and sort using fastapi-filter library, combined with some custom cases to estimate, how to move further.
Custom filters and sort in this example were implemented for the DeploymentFilter.
Why do we have these special cases
Filtering.
There are some fields we want to filter by, but they are not stored in the model (e.g. hub), so we request them from the Connect on every call. And fastapi-filter supports filtering only by model fields. We may decide to store them, but in this case we will need to follow all the changes with such objects and process them (add more events).
Ordering.
fastapi-filter library doesn't support sort by related fields (https://github.com/arthurio/fastapi-filter/blob/main/fastapi_filter/base/filter.py#L35). So there is no way to sort by product__name, e.g.
The further possible ways are the following:
Implement only filtering and sort by fields, that fastapi-filter allows us to (marked green here https://confluence.int.zone/display/CONNECT/XVS+extension+filtering+and+pagination+enhancement#XVSextensionfilteringandpaginationenhancement-Listoffieldstofilterby). Also there is a possibility to use sort by id instead of sort by name, not so convenient, but still useful.
Combine fastapi-filter with custom filtering and sort (like done in the PR for Deployment model). Looks like mess.
Make all the filtering and sort custom (remove fastapi-filter lib). Probably it will be more convenient to write own reusable functions or classes for filtering, appropriate for our cases.
Enhance the way we process custom cases. Likely it means to create own Filter class and to override its methods.
Personally I prefer to go 1st way. And if it won't be sufficient, discuss the list of required fields and come up with how to deal with them (maybe, for few fields it might fine to process them in a custom way). If we for sure need comprehensive solution, probably 3rd or 4th way is for us, need to investigate more.